Conversation
There was a problem hiding this comment.
Code Review
This pull request transitions the default output to Zod v4 schemas, adding a WithZodV3 option for legacy support. Key updates include refactoring struct conversion to use shape spreads, adopting specialized Zod v4 string format helpers, and implementing z.partialRecord for enum-keyed maps. Review feedback points out a logic error in struct conversion where named fields are incorrectly overridden by embedded spreads due to improper ordering; spreads should be emitted first to ensure outer struct fields take precedence. Additionally, a correction was suggested for the datetime format helper to use z.datetime() instead of z.iso.datetime() to align with Zod v4 conventions.
| merges := []string{} | ||
| embeddedFields := []string{} | ||
|
|
||
| fields := input.NumField() | ||
| for i := 0; i < fields; i++ { | ||
| field := input.Field(i) | ||
| optional := isOptional(field) | ||
| nullable := isNullable(field) | ||
|
|
||
| line, shouldMerge := c.convertField(field, indent+1, optional, nullable) | ||
| if field.Anonymous { | ||
| if c.zodV3 { | ||
| line, shouldMerge := c.convertEmbeddedFieldMerge(field, indent+1) | ||
| if shouldMerge { | ||
| merges = append(merges, line) | ||
| } else { | ||
| output.WriteString(line) | ||
| } | ||
| } else { | ||
| embeddedFields = append(embeddedFields, c.convertEmbeddedFieldSpread(field, indent+1)) | ||
| } | ||
| } else { | ||
| output.WriteString(c.convertNamedField(field, indent+1, optional, nullable)) | ||
| } | ||
| } | ||
|
|
||
| if !shouldMerge { | ||
| if !c.zodV3 { | ||
| for _, line := range embeddedFields { | ||
| output.WriteString(line) | ||
| } else { | ||
| merges = append(merges, line) | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar to getStructShape, there's a field ordering issue here for Zod v4 schemas. Named fields are written to the output before embedded fields are spread, which will cause properties from embedded structs to incorrectly override properties from the outer struct if they share the same name. The spreads should come first to ensure correct overriding behavior.
merges := []string{}
embeddedFields := []string{}
namedFields := []string{}
fields := input.NumField()
for i := 0; i < fields; i++ {
field := input.Field(i)
optional := isOptional(field)
nullable := isNullable(field)
if field.Anonymous {
if c.zodV3 {
line, shouldMerge := c.convertEmbeddedFieldMerge(field, indent+1)
if shouldMerge {
merges = append(merges, line)
} else {
output.WriteString(line)
}
} else {
embeddedFields = append(embeddedFields, c.convertEmbeddedFieldSpread(field, indent+1))
}
} else {
if c.zodV3 {
output.WriteString(c.convertNamedField(field, indent+1, optional, nullable))
} else {
namedFields = append(namedFields, c.convertNamedField(field, indent+1, optional, nullable))
}
}
}
if !c.zodV3 {
for _, line := range embeddedFields {
output.WriteString(line)
}
for _, line := range namedFields {
output.WriteString(line)
}
}| case "mongodb": | ||
| chunks = append(chunks, stringSchemaChunk{kind: "chain", text: fmt.Sprintf(".regex(/%s/)", mongodbRegexString)}) | ||
| case "datetime": | ||
| chunks = append(chunks, stringSchemaChunk{kind: "format", v4Base: "z.iso.datetime()", legacyChain: ".datetime()"}) |
There was a problem hiding this comment.
It seems there might be a typo here. The Zod v4 proposal for string formats suggests z.datetime() for datetime strings, not z.iso.datetime(). Was z.datetime() intended?
Also, it would be good to add a test case for the new datetime behavior in TestZodV4Defaults.
| chunks = append(chunks, stringSchemaChunk{kind: "format", v4Base: "z.iso.datetime()", legacyChain: ".datetime()"}) | |
| chunks = append(chunks, stringSchemaChunk{kind: "format", v4Base: "z.datetime()", legacyChain: ".datetime()"}) |
Summary
zen.WithZodV3()option for backwards compatibility with snapshot-based workflows or incremental migrationWithZodV3()usage exampleMigration differences (v3 → v4)
email,http_url,ipv4,uuid4,md5) now use Zod v4 helpers:z.email(),z.httpUrl(),z.ipv4(),z.uuid({ version: "v4" }),z.hash("md5")ip/ip_addremitz.union([z.ipv4(), z.ipv6()]).shapespreads instead of.merge(...)z.partialRecord(...)Test plan
WithZodV3()option produces previous v3 output🤖 Generated with Claude Code